Add caller-supplied policy authorization for firmware upgrade - #560
Open
dgarske wants to merge 1 commit into
Open
Add caller-supplied policy authorization for firmware upgrade#560dgarske wants to merge 1 commit into
dgarske wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends wolfTPM’s firmware-upgrade wrappers to support caller-supplied authorization sessions (e.g., policy sessions) for the vendor “firmware start” command, enabling platforms that gate upgrade behind custom platform hierarchy policies.
Changes:
- Added a
wolfTPM2_PolicyOR()wrapper to satisfy policy sessions viaTPM2_PolicyOR. - Added
wolfTPM2_FirmwareUpgradeHash_ex(..., startSession)and routed the legacywolfTPM2_FirmwareUpgradeHash()through it (preserving existing behavior whenstartSession == NULL). - Updated the ST33 firmware update example and documentation to demonstrate/describe policy-based authorization and a safe self-test flow.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wolftpm/tpm2_wrap.h | Declares new wolfTPM2_PolicyOR wrapper and wolfTPM2_FirmwareUpgradeHash_ex API with caller-supplied session support. |
| src/tpm2_wrap.c | Implements wolfTPM2_PolicyOR and adds the _ex firmware upgrade routing + vendor start-session plumbing. |
| examples/firmware/st33_fw_update.c | Adds --policytest self-test to validate policy-session + PolicyOR behavior without performing an upgrade. |
| examples/firmware/README.md | Documents advanced policy-based firmware start authorization and the new _ex API usage. |
dgarske
force-pushed
the
firmware_upgrade_policy_auth
branch
2 times, most recently
from
August 3, 2026 21:17
0b72b95 to
bda21be
Compare
dgarske
marked this pull request as ready for review
August 3, 2026 21:25
dgarske
force-pushed
the
firmware_upgrade_policy_auth
branch
from
August 4, 2026 21:36
bda21be to
01bc093
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (5)
src/tpm2_wrap.c:11213
- Severity: Critical (CWE-121/CWE-476). tpm2_ifx_firmware_start() builds a command in a fixed-size stack buffer (TPM_SHA512_DIGEST_SIZE) and copies manifest_hash using manifest_hash_sz without validating it. Through the public wolfTPM2_FirmwareUpgradeHash_ex() API a caller can pass a larger size (or a non-NULL size with NULL manifest_hash), causing a stack overflow or NULL dereference. Validate manifest_hash pointer and size against hashAlg before constructing the command.
int rc;
WOLFTPM2_SESSION tpmSession;
TPM_HANDLE sessionHandle = TPM_RH_NULL;
int ownSession = 0;
XMEMSET(&tpmSession, 0, sizeof(tpmSession));
src/tpm2_wrap.c:10739
- Severity: Medium (CWE-20). wolfTPM2_SetPrimaryPolicy() accepts authPolicySz > 0 with authPolicy == NULL, which would silently send an empty policy digest (size=0) while still setting hashAlg. This is an argument-validation bug that can result in unexpected policy changes or confusing TPM errors; fail fast when a non-zero size is provided without a buffer (and when clearing, enforce hashAlg==TPM_ALG_NULL implies authPolicySz==0).
This issue also appears on line 11207 of the same file.
if (dev == NULL) {
return BAD_FUNC_ARG;
}
if (authPolicySz > (word32)sizeof(in.authPolicy.buffer)) {
return BAD_FUNC_ARG;
}
examples/firmware/firmware_policy.c:184
- Severity: Low (CWE-476). firmware_policy_session_setup() dereferences dev and session without checking for NULL (it calls wolfTPM2_IsAlgSupported(dev, ...) and XMEMSET(session,...)). Even though this is example code, it’s a shared helper and should fail cleanly with BAD_FUNC_ARG on NULL inputs.
if (hsz == 0 || hsz > TPM_MAX_DIGEST_SIZE) {
return BAD_FUNC_ARG;
}
XMEMSET(session, 0, sizeof(*session));
XMEMSET(&orList, 0, sizeof(orList));
examples/firmware/st33_fw_update.c:425
- Severity: Low (CWE-772). In the ST33 example, sessionStarted is set to 1 before calling wolfTPM2_FirmwareUpgrade_ex(). If that call fails before FieldUpgradeStart is issued (e.g., comms/GetCapabilities failure), cleanup will skip UnloadHandle and leak the policy session. Consider only marking the session as “consumed” on overall success; if the TPM already consumed it on a later failure, UnloadHandle may fail harmlessly and is already ignored.
if (rc == 0) {
if (policyMode) {
printf("Using caller-supplied policy session\n");
/* session is handed to FieldUpgradeStart, which consumes it */
sessionStarted = 1;
}
rc = wolfTPM2_FirmwareUpgrade_ex(&dev,
fwinfo.manifest_buf, (uint32_t)fwinfo.manifest_bufSz,
TPM2_ST33_FwData_Cb, &fwinfo,
policyMode ? &policySession : NULL);
examples/firmware/ifx_fw_update.c:251
- Severity: Low (CWE-772). In the Infineon example, sessionStarted is set based only on caps.opMode before the upgrade call. If the upgrade fails before reaching FieldUpgradeStart (e.g., IO/GetCapabilities errors inside the library), cleanup will skip UnloadHandle and leak the policy session. Mark the session as “consumed” only when the overall call succeeds and the path actually reaches start (opMode != 0x03).
if (rc == 0) {
/* The upgrade hands the session to FieldUpgradeStart, which the TPM
* consumes as it enters upgrade mode. opMode 0x03 (finalize-only) does
* not reach start, so the session there is not consumed. */
if (policyMode) {
dgarske
force-pushed
the
firmware_upgrade_policy_auth
branch
from
August 5, 2026 01:01
01bc093 to
6a69bdb
Compare
dgarske
force-pushed
the
firmware_upgrade_policy_auth
branch
from
August 5, 2026 01:52
6a69bdb to
f29c7d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lets callers authorize the firmware-upgrade start command with their own platform-hierarchy policy (multi-branch PolicyOR, SHA-256/384/512) instead of the fixed library authorization. Backward compatible - no existing API changes.
Features
_exAPIs taking a caller-satisfied session:wolfTPM2_FirmwareUpgradeHash_ex,wolfTPM2_FirmwareUpgrade_ex,wolfTPM2_FirmwareUpgradeRecover_ex; the existing calls forward with a NULL session.wolfTPM2_PolicyOR,wolfTPM2_SetPrimaryPolicy,wolfTPM2_PolicyCommandCodeMake,wolfTPM2_IsAlgSupported.TPM_RS_PW.--policytestself-test and--policy/--policyor[--sha256|--sha384|--sha512]end-to-end modes; shared helpers factored intoexamples/firmware/firmware_policy.c.Fixes
wolfTPM2_PolicyORvalidates branch count and each branch digest size against its buffer (out-of-bounds read, CWE-125).TPM_CAP_ALGS) instead of failing withTPM_RC_SIZE.